Skip to content

accesslog: support stat-level AccessLogFilter in stats AccessLog#42676

Closed
TAOXUY wants to merge 4 commits intoenvoyproxy:mainfrom
TAOXUY:accessLogFilter
Closed

accesslog: support stat-level AccessLogFilter in stats AccessLog#42676
TAOXUY wants to merge 4 commits intoenvoyproxy:mainfrom
TAOXUY:accessLogFilter

Conversation

@TAOXUY
Copy link
Copy Markdown
Contributor

@TAOXUY TAOXUY commented Dec 17, 2025 •
edited
Loading

Commit Message: with the PR, we can filter out stats based on their on AccessLogFilter within accesslogger. The use case can be like

  • The gauge support can rely on this PR to add/subtract by defining multiple gauges with different log_type_filter(TcpConnectionStart, TcpConnectionEnd, etc).
  • we can have counters for connection error by using response_flag_filter(one of AccessLogFilter) isn't empty

Risk Level: no risk
Testing: added
Docs Changes: no need
Release Notes: no need as the stats AccessLog extension hasn't been released.
Platform Specific Features: no

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only Bot commented Dec 17, 2025

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #42676 was opened by TAOXUY.

see: more, trace.

@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Dec 17, 2025

@kyessenov @ggreenway

ggreenway
ggreenway reviewed Dec 18, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ggreenway ggreenway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with other access loggers, using

envoy/api/envoy/config/accesslog/v3/accesslog.proto

Line 38 in 9b74448

AccessLogFilter filter = 2;
to filter, and then having multiple instances of this access logger seems preferable. And that is already implemented.

Is there a use case where that wouldn't be sufficient?

/wait-any

@TAOXUY TAOXUY changed the title accesslog: support AccessLogFilter in stats AccessLog accesslog: support stat-level AccessLogFilter in stats AccessLog Dec 18, 2025
@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Dec 18, 2025

To be consistent with other access loggers, using

envoy/api/envoy/config/accesslog/v3/accesslog.proto

Line 38 in 9b74448

AccessLogFilter filter = 2;

to filter, and then having multiple instances of this access logger seems preferable. And that is already implemented.
Is there a use case where that wouldn't be sufficient?

/wait-any

Updated the description. PTAL

@TAOXUY
init
0292d8b 
Signed-off-by: Xuyang Tao <taoxuy@google.com>
@TAOXUY
fix release not
105d88c 
Signed-off-by: Xuyang Tao <taoxuy@google.com>
@TAOXUY
fix proto format
80af90e 
Signed-off-by: Xuyang Tao <taoxuy@google.com>
@repokitteh-read-only repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Dec 18, 2025
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only Bot commented Dec 18, 2025

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @agrawroh

🐱

Caused by: #42676 was synchronize by TAOXUY.

see: more, trace.

@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Dec 18, 2025

Sry for making a push by mistake and loop in so many peoples. I am not maintainers and I cannot unassign the reviewers unfortunately.

@ggreenway
Copy link
Copy Markdown
Member

ggreenway commented Dec 18, 2025

@wbpcode this feels weird to me from an API level because there's also a filter outside the access logger. But I can see how this would be useful in lowering the amount of config needed for some cases. WDYT?

@ggreenway ggreenway assigned ggreenway and kyessenov and unassigned agrawroh Dec 18, 2025
wbpcode
wbpcode reviewed Dec 19, 2025
View reviewed changes
Comment thread api/envoy/extensions/access_loggers/stats/v3/stats.proto Outdated
@ravenblackx ravenblackx removed their request for review December 19, 2025 17:37
@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Dec 19, 2025

I don't have a strong opinion on adding this API - will let API folks decide. I think you are not saving much on the costs yet (you could have two access logs with two filters, instead of one access log filter each with filter), unless I'm missing something. Generally, you want to evaluate predicate as early as possible, so it's calling for some nested matcher support down the line. Access log filters are difficult to use IMO, harder than it should be.

@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Dec 19, 2025

you could have two access logs with two filters, instead of one access log filter each with filter

That's a good point actually. Though feasible, it seems not ideal.

@ravenblackx
Copy link
Copy Markdown
Contributor

ravenblackx commented Dec 26, 2025

I believe this PR is waiting for API name change to be reversed.

/wait

@TAOXUY
fix proto name
bf33d5c 
Signed-off-by: Xuyang Tao <taoxuy@google.com>
@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Dec 29, 2025

I believe this PR is waiting for API name change to be reversed.

/wait

updated.

@ravenblackx
Copy link
Copy Markdown
Contributor

ravenblackx commented Dec 29, 2025 •
edited
Loading

updated.

No commit has been pushed to the PR. (The waiting label will automatically be removed when a change is pushed.)

Edit: even if github then shows the push as occurring in the past so that the sequence of events looks weird!

@ggreenway
Copy link
Copy Markdown
Member

ggreenway commented Jan 7, 2026

I still think this is an awkward API with two different levels of the same filtering. @wbpcode can you weigh in as the api-shepherd on this PR?

@kyessenov kyessenov removed their assignment Jan 8, 2026
@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Jan 14, 2026

I still think this is an awkward API with two different levels of the same filtering. @wbpcode can you weigh in as the api-shepherd on this PR?

Hi @ggreenway , not only

I still think this is an awkward API with two different levels of the same filtering. @wbpcode can you weigh in as the api-shepherd on this PR?

Another use case as I put above is that we can use response_flag_filter(one of AccessLogFilter) to filter out connections with errors.

@TAOXUY
Copy link
Copy Markdown
Contributor Author

TAOXUY commented Jan 14, 2026

Close for now, I will make another PR for filter tag value.

@TAOXUY TAOXUY closed this Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ggreenway ggreenway ggreenway left review comments

@wbpcode wbpcode wbpcode left review comments

@kyessenov kyessenov Awaiting requested review from kyessenov kyessenov is a code owner

@RyanTheOptimist RyanTheOptimist Awaiting requested review from RyanTheOptimist

@abeyad abeyad Awaiting requested review from abeyad

@danzh2010 danzh2010 Awaiting requested review from danzh2010

@nezdolik nezdolik Awaiting requested review from nezdolik

@tonya11en tonya11en Awaiting requested review from tonya11en

@fredyw fredyw Awaiting requested review from fredyw

@mattklein123 mattklein123 Awaiting requested review from mattklein123

@botengyao botengyao Awaiting requested review from botengyao

@yanavlasov yanavlasov Awaiting requested review from yanavlasov

@agrawroh agrawroh Awaiting requested review from agrawroh

@KBaichoo KBaichoo Awaiting requested review from KBaichoo

@adisuissa adisuissa Awaiting requested review from adisuissa

Assignees

@ggreenway ggreenway

@wbpcode wbpcode

Labels

api deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@TAOXUY @ggreenway @kyessenov @ravenblackx @wbpcode @agrawroh